Avatars + Spell results by igor47 · Pull Request #56 · igor47/csheet

igor47 · 2025-11-13T22:23:47Z

address most existing feedback re: showing better spell-result details and also avatar display.

fixes #52

Copilot

Pull Request Overview

This PR enhances the avatar system by migrating from a single avatar per character to support multiple avatars with cropping capabilities, and improves spell casting feedback by displaying detailed attack/damage information.

Key Changes:

Introduces a new character_avatars table allowing multiple avatars per character with crop percentages stored as normalized values (0-1)
Adds comprehensive spell result details including attack bonuses, save DCs, damage formulas, and scaling explanations
Implements avatar gallery, lightbox, and cropping UI using Cropper.js v2 web components

Reviewed Changes

Copilot reviewed 28 out of 31 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
`migrations/20251112060352_create_character_avatars.sql`	Creates new table for storing multiple avatars with crop data and primary flag
`migrations/20251112060400_migrate_existing_avatars.sql`	Migrates existing avatar_id values to new character_avatars table
`migrations/20251112060500_remove_avatar_id_from_characters.sql`	Removes deprecated avatar_id column from characters table
`src/db/character_avatars.ts`	New database layer for avatar CRUD operations with crop coordinate management
`src/db/characters.ts`	Removes avatar_id from Character schema
`src/services/updateAvatar.ts`	Updates to create avatar records in new table instead of updating character
`src/services/castSpell.ts`	Adds attack info computation with damage scaling and save DC details
`src/services/computeCharacter.ts`	Loads all character avatars with URLs for display
`src/components/AvatarDisplay.tsx`	New component rendering avatars with CSS-based soft cropping
`src/components/AvatarGallery.tsx`	Gallery view for managing multiple avatars with set primary/crop/delete actions
`src/components/AvatarLightbox.tsx`	Full-size avatar viewer with navigation
`src/components/AvatarCropper.tsx`	Cropper UI for adjusting avatar crops
`src/components/SpellCastResult.tsx`	Enhanced to display attack rolls, save DCs, damage formulas, and scaling
`src/components/CharacterInfo.tsx`	Updated to use new AvatarDisplay component
`src/routes/character.tsx`	Adds routes for avatar gallery, lightbox, cropping, and management
`static/avatar-cropper.js`	Client-side logic for Cropper.js integration and coordinate calculations
`static/htmx-auth.js`	Handles HTMX redirects for auth error responses
`static/styles.css`	Generalizes avatar styling from IDs to classes for reusability
`package.json`	Adds cropperjs dependency and postinstall script

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/services/castSpell.ts

src/components/UpdateAvatarForm.tsx

static/avatar-cropper.js

src/routes/character.tsx

Copilot · 2025-11-13T22:28:51Z

src/routes/character.tsx

+  // Check if this is the only/primary avatar
+  const count = await CharacterAvatars.countByCharacterId(getDb(c), characterId)
+  if (count === 1) {
+    return c.json({ error: "Cannot delete the only avatar" }, 400)
+  }


The error message says "Cannot delete the only avatar" but characters can exist without avatars (as evidenced by the placeholder image support). This validation seems inconsistent with the system's design. Consider allowing deletion of the last avatar or clarifying the business requirement.

Suggested change

// Check if this is the only/primary avatar

const count = await CharacterAvatars.countByCharacterId(getDb(c), characterId)

if (count === 1) {

return c.json({ error: "Cannot delete the only avatar" }, 400)

}

// Allow deletion even if this is the only avatar

Copilot · 2025-11-13T22:28:52Z

src/services/castSpell.ts

+          const dicePerLevel = scaledDice.length - baseDice.length
+          scalingExplanation = `Base: ${baseFormula}, +${dicePerLevel}d${scaledDice[0]} per level above ${spell.level} = ${scaledFormula}`


The calculation dicePerLevel assumes all dice in the array have the same die size (accessing scaledDice[0]), but if the spell has mixed dice types, this will produce an incorrect explanation. Consider handling cases where dice types vary or documenting this assumption.

Suggested change

const dicePerLevel = scaledDice.length - baseDice.length

scalingExplanation = `Base: ${baseFormula}, +${dicePerLevel}d${scaledDice[0]} per level above ${spell.level} = ${scaledFormula}`

// Compute dice added per level, grouped by die type

const diceCount = (arr: number[]) =>

arr.reduce((acc, die) => {

acc[die] = (acc[die] || 0) + 1

return acc

}, {} as Record<number, number>)

const baseDiceCount = diceCount(baseDice)

const scaledDiceCount = diceCount(scaledDice)

// Find difference

const addedDice: string[] = []

for (const die in scaledDiceCount) {

const added = scaledDiceCount[die] - (baseDiceCount[die] || 0)

if (added > 0) {

addedDice.push(`+${added}d${die}`)

}

}

const addedDiceStr = addedDice.length > 0 ? addedDice.join(", ") : "no extra dice"

scalingExplanation = `Base: ${baseFormula}, ${addedDiceStr} per level above ${spell.level} = ${scaledFormula}`

src/routes/character.tsx

Copilot · 2025-11-13T22:28:53Z

migrations/20251112060400_migrate_existing_avatars.sql

+-- Migrate existing avatar_id from characters table to character_avatars table
+-- Using md5 hash of character_id + timestamp to generate unique IDs (ULID-like)
+INSERT INTO character_avatars (id, character_id, upload_id, is_primary, created_at, updated_at)
+SELECT
+  SUBSTRING(MD5(id || EXTRACT(EPOCH FROM CURRENT_TIMESTAMP)::TEXT) FROM 1 FOR 26) as id,


The ID generation using SUBSTRING(MD5(...)) in the migration may not guarantee uniqueness across all records, especially if multiple characters are migrated in quick succession. While unlikely to collide in practice, consider using a proper ULID generation function or UUID if available in PostgreSQL to ensure uniqueness.

Suggested change

-- Migrate existing avatar_id from characters table to character_avatars table

-- Using md5 hash of character_id + timestamp to generate unique IDs (ULID-like)

INSERT INTO character_avatars (id, character_id, upload_id, is_primary, created_at, updated_at)

SELECT

SUBSTRING(MD5(id || EXTRACT(EPOCH FROM CURRENT_TIMESTAMP)::TEXT) FROM 1 FOR 26) as id,

-- Ensure pgcrypto extension is enabled for gen_random_uuid()

CREATE EXTENSION IF NOT EXISTS "pgcrypto";

-- Migrate existing avatar_id from characters table to character_avatars table

-- Using md5 hash of character_id + timestamp to generate unique IDs (ULID-like)

INSERT INTO character_avatars (id, character_id, upload_id, is_primary, created_at, updated_at)

SELECT

gen_random_uuid() as id,

Copilot · 2025-11-13T22:28:53Z

migrations/20251112060352_create_character_avatars.sql

+  updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+  UNIQUE(character_id, upload_id)


[nitpick] The UNIQUE constraint on (character_id, upload_id) prevents the same upload from being used multiple times for the same character, even with different crop settings. This might be intentional, but it limits flexibility. Consider whether users should be able to create multiple avatars from the same upload with different crops.

Suggested change

updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,

UNIQUE(character_id, upload_id)

updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP

Copilot · 2025-11-13T22:28:54Z

static/avatar-cropper.js

+      }
+
+      // we might have extra space on the sides if the image is narrow
+      const canvasExtraV = (canvasW - renderedW) / 2


Avoid automated semicolon insertion (90% of all statements in the enclosing function have an explicit semicolon).

Suggested change

const canvasExtraV = (canvasW - renderedW) / 2

const canvasExtraV = (canvasW - renderedW) / 2;

both afterwards for attacks, and reed learns the stuff also

we do the db work and some initial UI work, but the UI doesn't fully function

we actually manage to crop the images and save the cropped box details

we use CSS to soft-crop the uploaded images

mostly, similar props as the others

infra: give deploy user pulumi KMS access

bf553db

igor47 requested a review from Copilot November 13, 2025 22:23

Copilot started reviewing on behalf of igor47 November 13, 2025 22:24 View session

Copilot finished reviewing on behalf of igor47 November 13, 2025 22:26

Copilot AI reviewed Nov 13, 2025

View reviewed changes

igor47 added 8 commits November 13, 2025 16:08

feat: show spell cast results

02b75b4

both afterwards for attacks, and reed learns the stuff also

fix: correct location for htmx-auth file

778e631

feat: initial stab at multiple avatars

da007a8

we do the db work and some initial UI work, but the UI doesn't fully function

feat: character avatar lightbox

8f9fb6e

feat: correct cropping logic

f9eb5f9

we actually manage to crop the images and save the cropped box details

feat: correct avatar display post-scaling

3df9f61

we use CSS to soft-crop the uploaded images

refactor: only a single avatar upload endpoint

75e0624

refactor: avatar cropper consistent with other components

58a4ce1

mostly, similar props as the others

igor47 force-pushed the igor/avatar-spell-results branch from 43a5628 to 58a4ce1 Compare November 14, 2025 01:13

igor47 merged commit 06df220 into main Nov 14, 2025
2 checks passed

igor47 deleted the igor/avatar-spell-results branch November 14, 2025 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avatars + Spell results#56

Avatars + Spell results#56
igor47 merged 9 commits intomainfrom
igor/avatar-spell-results

igor47 commented Nov 13, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Uh oh!

Copilot AI Nov 13, 2025

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Uh oh!

Copilot AI Nov 13, 2025

Uh oh!

Copilot AI Nov 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const dicePerLevel = scaledDice.length - baseDice.length
		scalingExplanation = `Base: ${baseFormula}, +${dicePerLevel}d${scaledDice[0]} per level above ${spell.level} = ${scaledFormula}`

-          const dicePerLevel = scaledDice.length - baseDice.length
-          scalingExplanation = `Base: ${baseFormula}, +${dicePerLevel}d${scaledDice[0]} per level above ${spell.level} = ${scaledFormula}`
+          // Compute dice added per level, grouped by die type
+          const diceCount = (arr: number[]) =>
+            arr.reduce((acc, die) => {
+              acc[die] = (acc[die] || 0) + 1
+              return acc
+            }, {} as Record<number, number>)
+          const baseDiceCount = diceCount(baseDice)
+          const scaledDiceCount = diceCount(scaledDice)
+          // Find difference
+          const addedDice: string[] = []
+          for (const die in scaledDiceCount) {
+            const added = scaledDiceCount[die] - (baseDiceCount[die] || 0)
+            if (added > 0) {
+              addedDice.push(`+${added}d${die}`)
+            }
+          }
+          const addedDiceStr = addedDice.length > 0 ? addedDice.join(", ") : "no extra dice"
+          scalingExplanation = `Base: ${baseFormula}, ${addedDiceStr} per level above ${spell.level} = ${scaledFormula}`

		updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
		UNIQUE(character_id, upload_id)

	const canvasExtraV = (canvasW - renderedW) / 2
	const canvasExtraV = (canvasW - renderedW) / 2;

Conversation

igor47 commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

igor47 commented Nov 13, 2025 •

edited

Loading